Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Avoid leaving subdoc defaults on top-level doc when setting subdocument to same value #14728

Merged
merged 3 commits into from
Jul 9, 2024

Conversation

vkarpov15
Copy link
Collaborator

Summary

The cause of #14722 is that, when creating a new subdocument, Mongoose first applies defaults to the subdocument, and those defaults bubble up to the top-level document. Which leaves any default paths in default state on the top-level document, even if we're setting the subdocument to the exact same value.

This PR makes it so that we skip parent change tracking on defaults when creating a completely new subdocument - the idea is, if we're creating a new subdocument, we'll be setting the subdocument to the top-level document, so we will track the change as "set the subdocument" rather than "set the subdocument, and also set this default on the subdocument."

There's also some followup work I want to do related to #4145: #4145 succeeds because we leave document array underneath a nested path in default state, even though we modify the document array.

Examples

Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont fully understand what is fixed here, but the code looks good to me.

@vkarpov15
Copy link
Collaborator Author

Lol. To put it another way, the issue is that, when you do the following, user.$isDefault('updater._id') will be true even though updater was set to the exact same value it was already.

const getUser = () => ({
    _id: new mongoose.Types.ObjectId("66852317541ef0e22ae5214c"),
    name: "blowfishlol",
    email: "blowfish@test.com"
});

const { _id } = await TestModel.create({ name: 'foobar', updater: getUser() });

const user = await TestModel.findById(_id).orFail();
user.set('updater', getUser());

user.$isDefault('updater._id'); // true without this PR, which shouldn't happen since we set `updater` to the same value

@vkarpov15 vkarpov15 merged commit bb7e929 into master Jul 9, 2024
45 of 47 checks passed
@vkarpov15 vkarpov15 added this to the 8.4.6 milestone Jul 9, 2024
@hasezoey hasezoey deleted the vkarpov15/gh-14722 branch July 10, 2024 12:03
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants